Make opensearch_security.multitenancy.tenants.preferred configurable dynamically via security config api#5986
Conversation
…nfo API Signed-off-by: Vishnutheep B <vishnutheep@gmail.com>
Signed-off-by: Vishnutheep B <vishnutheep@gmail.com>
itsmevichu
requested review from
DarshitChanpura,
RyanL1997,
cwperks,
derek-ho,
nibix,
reta,
shikharj05 and
willyborankin
as code owners
Signed-off-by: Vishnutheep B <vishnutheep@gmail.com>
Signed-off-by: Vishnutheep B <vishnutheep@gmail.com>
|
@itsmevichu thank you for this PR. Can you please fix the CHANGELOG conflict? Apologies for not running the checks earlier. They are running now. Please look for any failures if some of the checks come back red. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5986 +/- ##
==========================================
+ Coverage 73.79% 73.90% +0.10%
==========================================
Files 440 440
Lines 27259 27371 +112
Branches 4052 4076 +24
==========================================
+ Hits 20117 20229 +112
+ Misses 5227 5218 -9
- Partials 1915 1924 +9
🚀 New features to boost your workflow:
|
Signed-off-by: Vishnutheep B <vishnutheep@gmail.com>
itsmevichu
force-pushed
the
feature/gh-5969
branch
from
6dc4f6f to
8f12076
Compare
|
The failing CI checks seems irrelevant. |
| } | ||
|
|
||
| private ValidationResult<JsonNode> nullValuesInArrayValidator(final JsonNode jsonContent) { | ||
| for (final Map.Entry<String, DataType> allowedKey : validationContext.allowedKeys().entrySet()) { |
There was a problem hiding this comment.
nit: I think key name is all that matters here so idk if we need this change. That being said, is it enforced that allowedKeyWithConfig is strictly a subset of allowedKeys?
There was a problem hiding this comment.
I understand, we don’t need the DataType; just the key name is sufficient, and I’ll update the changes.
And allowedKeysWithConfig isn’t required to be a strict subset of allowedKeys. If allowedKeysWithConfig is present, it takes precedence; otherwise, we fallback to allowedKeys. Going forward, for any new API we define, we can start using allowedKeysWithConfig directly, which will make it easier when we eventually remove allowedKeys completely.
|
@itsmevichu the changes in this PR look good to me. Only question I have is if We have had some bwc issues with the security index in mixed clusters if the value is set in a mixed cluster and the older version of nodes is not aware of this value. As long as its not auto-persisted when new nodes join the cluster then its reasonably backwards compatible as long as a security admin does not try to set it in a mixed cluster. |
|
Note: backend functionality helps get the setting, but security-dashboards-plugin needs a change to consume this dynamic setting. |
2 participants
Description
This PR proposes making
opensearch_security.multitenancy.tenants.preferredconfigurable dynamically via the Security configuration API in OpenSearch, instead of being solely controlled via opensearch_dashboards.ymlBefore:
preferred_tenantswas not part of config APIs or/dashboardsinfo.After:
preferred_tenantscan be set via tenancy config API, is persisted inConfigV7.Kibana, returned in/dashboardsinfo.Issues Resolved
#5969
Testing
MultiTenancyConfigApiTestandConfigV7Testforpreferred_tenants. :Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.